Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travis-CI testing #474

Merged
merged 9 commits into from Mar 8, 2013
Merged

Travis-CI testing #474

merged 9 commits into from Mar 8, 2013

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Mar 6, 2013

This pull request adds Travis-CI continuous integration.
Here is the current build status: Build Status

Ouch. 10/137 is not so good. The bright side is that it seems to be a few key issues, such as priv.settings.data being undefined occasionally, and that the behavior of the generated _SpecRunner.html is the same as it was before in Chrome, though FF may have picked up a bit of a flutter.

I hope this helps in getting to at least a baseline for doing automated testing, but due to the selection of browsers supported, this won't cover everything. But who knows? Maybe a headless IE7 can be run under Wine or something.

@warpech
Copy link
Member

warpech commented Mar 7, 2013

Looking at your branch, seems that it is safe to merge this pull request before I merge #471. Can you confirm?

@warpech
Copy link
Member

warpech commented Mar 7, 2013

I examined this pull request and it looks really neat. Just 2 questions:

  1. why did you have to add grunt-cli to package.json? Is that something Travis needs and there is no way to use it globally in Travis? This guide says that the correct way is to install it globally. Readme says that local installation is not officially supported. I see that other people had the same problem already.

  2. Is JqueryHandsontableRunner.tmpl really required? Can't grunt-contrib-jasmine use SpecRunner.html?

Thanks, it looks really exciting to be one step closer to full test automation :)

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Mar 7, 2013

Looking at your branch, seems that it is safe to merge this pull request before I merge #471. Can you confirm?

Yep, this has nothing to do with #471. Also just rebased onto 0.8.10.

As I started digging into testing the stuff in #471, I thought it would make sense to get some of this going.

grunt-cli

Yup, researched it up, and now have it in the before_script chunk. good call.

JqueryHandsontableRunner.tmpl

Just trying to "do stuff the X way," but I'm not sure whether X is Node, Grunt or Just Some Guy. Storing the list of dependencies, built source and specs in the checked-in SpecRunner.html, and the entirety of jasmine, seems like a lot of duplication of data, but makes sense if the current target is real browsers, and not Phantom. The generated _SpecRunner.html can be left around after running grunt test, and hence could be checked in for meeting the case of testing against real browsers.

On a related note: I'd like to see the ability to test the un-concated src/ as well, as right now chasing down a problem requires digging into the generated jquery-handsontable.js, then string matching to where that appears in src/wherever.js. Some changes would have to happen in intro. I experimented with this, but couldn't get it to work. Thoughts?

@warpech warpech merged commit 9ffda82 into handsontable:master Mar 8, 2013
@warpech
Copy link
Member

warpech commented Mar 8, 2013

Thanks, so I merged your pull request. I have the same result as you had: https://travis-ci.org/warpech/jquery-handsontable

I will work on fixing that during the next days.

JqueryHandsontableRunner.tmpl

I am just trying to keep things DRY so I wanted to avoid having duplicate suite definition in the working tree. Can't SpecRunner.html be used by grunt-contrib-jasmine? I will look into that.

I think it is needless to say that running test suites in real browsers is as much important as in Phantom - to ensure cross-browser compatibility.

On a related note: I'd like to see the ability to test the un-concated src/ as well, as right now chasing down a problem requires digging into the generated jquery-handsontable.js, then string matching to where that appears in src/wherever.js. Some changes would have to happen in intro. I experimented with this, but couldn't get it to work. Thoughts?

Have you tried loading the whole list of scripts in the SpecRunner instead of jquery.handsontable.js? If you omit intro and outro, it might just work (I haven't tried that though).

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Mar 8, 2013

Into has the namespace... Could that move inside core?
On Mar 7, 2013 8:22 PM, "Marcin Warpechowski" notifications@github.com
wrote:

Thanks, so I merged your pull request. I have the same result as you had:
https://travis-ci.org/warpech/jquery-handsontable

I will work on fixing that during the next days.

JqueryHandsontableRunner.tmpl

I am just trying to keep things DRY so I wanted to avoid having duplicate
suite definition in the working tree. Can't SpecRunner.html be used by
grunt-contrib-jasmine? I will look into that.

I think it is needless to say that running test suites in real browsers is
as much important as in Phantom - to ensure cross-browser compatibility.

On a related note: I'd like to see the ability to test the un-concated
src/ as well, as right now chasing down a problem requires digging into the
generated jquery-handsontable.js, then string matching to where that
appears in src/wherever.js. Some changes would have to happen in intro. I
experimented with this, but couldn't get it to work. Thoughts?

Have you tried loading the whole list of scripts in the SpecRunner instead
of jquery.handsontable.js? If you omit intro and outro, it might just
work (I haven't tried that though).


Reply to this email directly or view it on GitHubhttps://github.com//pull/474#issuecomment-14597489
.

@warpech
Copy link
Member

warpech commented Mar 8, 2013

Ah right...

Maybe it can, but then it may be executed too late for some incompatible plugins... (the inside is executed on $(document).ready) Beter avoid that.

Maybe temporarily just add the namespace to the SpecRunner:

<script>
var Handsontable = { //class namespace
  extension: {}, //extenstion namespace
  helper: {} //helper namespace
};
</script>

Before any other script.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Mar 8, 2013

or duplicate it... I'll try some things. I did run into some problems with
some load order stuff with discrete files... Ill try on a fresh branch.
On Mar 7, 2013 9:08 PM, "Marcin Warpechowski" notifications@github.com
wrote:

Ah right...

Maybe it can, but then it may be executed too late for some incompatible
plugins... (the inside is executed on $(document).ready) Beter avoid
that.

Maybe temporarily just add the namespace to the SpecRunner:

<script> var Handsontable = { //class namespace extension: {}, //extenstion namespace helper: {} //helper namespace }; </script>

Before any other script.


Reply to this email directly or view it on GitHubhttps://github.com//pull/474#issuecomment-14598947
.

@warpech
Copy link
Member

warpech commented Mar 12, 2013

Now Travis CI is almost passing :) See the release notes for 0.8.13 in the changelog.

@warpech
Copy link
Member

warpech commented Mar 12, 2013

BTW I managed to get rid of the file JqueryHandsontableRunner.tmpl by specifying CSS dependencies in the Grunfile.js. It wasn't easy to get through all the docs but now it works

@bollwyvl
Copy link
Contributor Author

Man, I thought I tried styles. Looking great!

On Mon, Mar 11, 2013 at 9:51 PM, Marcin Warpechowski <
notifications@github.com> wrote:

BTW I managed to get rid of the file JqueryHandsontableRunner.tmpl by
specifying CSS dependencies in the Grunfile.js. It wasn't easy to get
through all the docs but now it works


Reply to this email directly or view it on GitHubhttps://github.com//pull/474#issuecomment-14754378
.

@bollwyvl bollwyvl deleted the travis-testing branch March 13, 2013 18:19
@bollwyvl
Copy link
Contributor Author

Just saw the green light on the home page. Hooray!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants